Allow using banked Codex reset credits#1690
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 1, 2026, 3:50 PM ET / 19:50 UTC. Summary Reproducibility: not applicable. this is a feature PR for a new provider-side reset action, not a broken existing behavior. The PR includes screenshots and request-stub tests for the intended flow, while live maintainer QA intentionally avoided spending another credit. Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Keep this PR open until the owner chooses the product/auth boundary: either split and land read-only inventory/expiry work, or explicitly approve the consume action with final exact-head proof and validation. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a feature PR for a new provider-side reset action, not a broken existing behavior. The PR includes screenshots and request-stub tests for the intended flow, while live maintainer QA intentionally avoided spending another credit. Is this the best way to solve the issue? Unclear until owner approval: the implementation is mechanically plausible after account-scoping and expiry fixes, but the safest product path may be to keep CodexBar read-only or split the read-only reset inventory from the consume action. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c3d33308ac06. Label changesLabel justifications:
Evidence reviewedSecurity concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
brahimhamichan
force-pushed
the
codex-reset-credits
branch
from
d9eb894 to
855ce01
Compare
clawsweeper
Bot
added
rating: 🧂 unranked krab
Screenshots of the implementation:
|
brahimhamichan
force-pushed
the
codex-reset-credits
branch
from
855ce01 to
53b7507
Compare
clawsweeper
Bot
added
proof: 📸 screenshot
brahimhamichan
force-pushed
the
codex-reset-credits
branch
from
53b7507 to
35fbd30
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35fbd302d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
brahimhamichan
force-pushed
the
codex-reset-credits
branch
from
35fbd30 to
bb7a07f
Compare
Before and after using the use rest buttonBefore: [had 3 rests] After: [Left with 2 resets] |
brahimhamichan
force-pushed
the
codex-reset-credits
branch
from
bb7a07f to
38c6a98
Compare
clawsweeper
Bot
added
proof: sufficient
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
brahimhamichan
force-pushed
the
codex-reset-credits
branch
from
38c6a98 to
0d2d839
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
brahimhamichan
force-pushed
the
codex-reset-credits
branch
from
4b95081 to
f84a022
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
Maintainer overlap audit against current This PR is not superseded by current reset-credit work. It uniquely adds the confirmed consume action, account-scoped POST, request id, UI confirmation, and completion/error handling. Focused exact-head tests are green: I am not landing an undocumented write that consumes a finite credit without explicit product/auth approval. If approved, the PR should be rebased only after the active refresh/menu/localization work it overlaps (#1800, #1776, #1736, #1803), then receive fresh packaged confirmation-flow proof and an exact-head CI pass. Preserve contributor credit; there is unique work here, so do not evidence-close it as superseded. Recommendation: keep open and request the owner decision on whether CodexBar should expose credit consumption at all. No merge from this review. |
|
Maintainer repair complete at exact head Proof:
All existing review threads are resolved and their account-scoping concerns are covered by current tests. Maintainer QA intentionally did not invoke the real consume endpoint. Product decision remains explicit in the PR body. My recommendation is do not land the provider-side consume action yet; split the read-only expiration inventory/notification work if that UI is wanted independently. Overlap audit (2026-07-01):
Per the active-overlap gate, I am not rebasing #1690 until #1736 lands. That avoids manufacturing a temporary resolution against a moving menu/refresh surface. |
There was a problem hiding this comment.
💡 Codex Review
Freeze reset-credit decoder test time
This fixture expires the expected RateLimitResetCredit_earlier on July 12, 2026, but _decodeRateLimitResetCreditsForTesting stamps the snapshot with Date(). After July 12, 2026 the expected next expiring credit becomes the later fixture (and after July 18 it becomes nil), so the test suite starts failing without any code change; use a fixed injected clock or construct the snapshot with an explicit updatedAt.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
clawsweeper
Bot
added
rating: 🦐 gold shrimp
steipete
force-pushed
the
codex-reset-credits
branch
from
00da631 to
c248d0c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c248d0cce2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| public func availableCredits(at date: Date) -> [CodexRateLimitResetCredit] { | ||
| self.credits.filter { credit in | ||
| credit.status == .available && (credit.expiresAt ?? .distantPast) > date |
There was a problem hiding this comment.
When the API returns an available reset credit with expires_at: null, this filter maps the missing expiry to .distantPast, so the new count/action paths treat it as already expired. Since CodexRateLimitResetCredit.expiresAt is optional and the UI added a “No expiry” rendering path, those credits should remain visible and consumable instead of disappearing from the menu/settings; only non-nil expiries that are at or before date should be filtered out.
Useful? React with 👍 / 👎.
|
@clawsweeper re-review Pushed Checks run:
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Pushed final head Checks run on the merged tree:
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
the
merge-risk: 🚨 security-boundary
2 participants
Owner decision requested
Decision: Should CodexBar spend a finite provider-side Codex reset credit through an authenticated
POST, or remain read-only and limit this work to reset inventory/expiry notifications?Tradeoff: One-click consumption is convenient, confirms before spending, refreshes afterward, and scopes authentication to the selected account. It remains an irreversible provider-side write through a private endpoint contract; confirmation does not remove endpoint-stability, authorization, or accidental-spend risk.
Recommendation: Do not land the consume action yet. Keep CodexBar read-only until the provider contract is documented or explicitly approved. If immediate progress is desired, split and land the read-only inventory/notification work; retain this branch as the reviewed write-action option.
Summary
Addresses #1502 only if the write action is accepted; the issue stays open while this PR is held.
Validation
c248d0cce2acf07d36b042bc4c41c0b553aba427origin/main; contributor authorship preservedmake test: 44/44 shards passedmake check: passedCODEXBAR_SIGNING=adhoc ./Scripts/package_app.sh debug: passedMaintainer QA intentionally did not invoke the live consume endpoint.
Existing contributor UI proof